feat(ci): add entity-registry determinism check — fixes #758 - #765
Conversation
Re-runs `populate-entity-registry.js` against the current source tree and diffs the output against the committed `.aiox-core/data/entity-registry.yaml`. Fails the build if any non-volatile fields (dependencies, usedBy, structural) differ between the committed registry and a clean regen. Catches the category of drift that produced #754 (false alarm): a regen run against a partially-edited working tree silently producing a registry that doesn't match what the algorithm would produce on the same source. ## Implementation - `scripts/validate-registry-determinism.js` — snapshots the committed registry, runs the populator (which mutates the file in place), reads back, always restores the snapshot in `finally` so the working tree is preserved regardless of outcome, strips volatile fields (lastUpdated, lastVerified, checksum) before deep-comparing, and emits an actionable error listing the specific entries that diverged on dependencies or usedBy. - `package.json` — `validate:registry-determinism` npm script wired to the new script. - `.github/workflows/ci.yml` — new `registry-determinism` job in the existing `changes.outputs.code` gate (.aiox-core/**, scripts/**, etc), runs in ~5 min, blocks merge on drift. ## Tested locally - Clean main: PASSES (820 entities, structure matches). - Induced drift (manually zeroed deps of one task): FAILS with "[tasks] analyze-brownfield: dependencies differ" and remediation hint. ## What this does NOT catch - Drift inside other generated files (install-manifest.yaml, IDE projections) — covered by `validate:manifest` and `validate:ide-sync` already. - Algorithmic regressions in the populator itself — those should land via the regular dev/test loop; this gate only verifies deterministic output given the committed source state. Closes #758 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a Node.js validator that re-runs the entity registry populator, strips volatile metadata, and deterministically compares the regenerated registry to the committed ChangesRegistry Determinism Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Coverage ReportCoverage report not available
Generated by PR Automation (Story 6.1) |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 321-347: The new job "registry-determinism" must be added to the
overall validation/merge gate by making "validation-summary" depend on it and
include its failure in the summary; update the "validation-summary" job's needs
list to include "registry-determinism" (or include it among the jobs whose
results are aggregated) so that any failure in registry-determinism blocks the
merge gate, and ensure the summary aggregation logic references
"registry-determinism" when computing pass/fail state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c04e6058-773c-4129-8361-072d5f23c934
📒 Files selected for processing (3)
.github/workflows/ci.ymlpackage.jsonscripts/validate-registry-determinism.js
CodeRabbit pointed out that the new `registry-determinism` job was running but not aggregated by `validation-summary`, so a determinism failure wouldn't block the merge gate. Added to `needs` list and to the FAILED check logic with a clear error message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addressed in commit 1506dd4 — registry-determinism is now wired into validation-summary needs list and FAILED-check logic. Superseded by APPROVED review 4309972894.
…ynkraAI#765) * feat(ci): add entity-registry determinism check [SynkraAI#758] Re-runs `populate-entity-registry.js` against the current source tree and diffs the output against the committed `.aiox-core/data/entity-registry.yaml`. Fails the build if any non-volatile fields (dependencies, usedBy, structural) differ between the committed registry and a clean regen. Catches the category of drift that produced SynkraAI#754 (false alarm): a regen run against a partially-edited working tree silently producing a registry that doesn't match what the algorithm would produce on the same source. ## Implementation - `scripts/validate-registry-determinism.js` — snapshots the committed registry, runs the populator (which mutates the file in place), reads back, always restores the snapshot in `finally` so the working tree is preserved regardless of outcome, strips volatile fields (lastUpdated, lastVerified, checksum) before deep-comparing, and emits an actionable error listing the specific entries that diverged on dependencies or usedBy. - `package.json` — `validate:registry-determinism` npm script wired to the new script. - `.github/workflows/ci.yml` — new `registry-determinism` job in the existing `changes.outputs.code` gate (.aiox-core/**, scripts/**, etc), runs in ~5 min, blocks merge on drift. ## Tested locally - Clean main: PASSES (820 entities, structure matches). - Induced drift (manually zeroed deps of one task): FAILS with "[tasks] analyze-brownfield: dependencies differ" and remediation hint. ## What this does NOT catch - Drift inside other generated files (install-manifest.yaml, IDE projections) — covered by `validate:manifest` and `validate:ide-sync` already. - Algorithmic regressions in the populator itself — those should land via the regular dev/test loop; this gate only verifies deterministic output given the committed source state. Closes SynkraAI#758 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): wire registry-determinism into validation-summary gate [SynkraAI#758] CodeRabbit pointed out that the new `registry-determinism` job was running but not aggregated by `validation-summary`, so a determinism failure wouldn't block the merge gate. Added to `needs` list and to the FAILED check logic with a clear error message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
New CI step that re-runs `populate-entity-registry.js` on every PR touching the IDS sources and fails if the committed `entity-registry.yaml` diverges from a clean regen output. Closes the category of drift that produced the #754 false alarm.
Why this exists
PR #752 inadvertently committed a registry generated against a partially-edited working tree, with `dependencies: []` for 5 agents and 4 tasks whose markdown sources still declared those deps. The drift was invisible until issue #754 was filed (and then refuted — see #754 closure comment), demonstrating that regen-vs-committed drift can corrupt the registry without anyone noticing until impact analysis goes wrong.
This gate catches that mechanically before merge.
Components
`scripts/validate-registry-determinism.js`: snapshots committed registry → runs populator → reads regen output → ALWAYS restores snapshot in `finally` (working tree preserved regardless of outcome) → strips volatile fields (`lastUpdated`, `lastVerified`, `checksum`) → deep-compares via sorted recursive stringify (so map ordering doesn't yield false positives) → emits actionable per-entry diff on failure.
`package.json`: `npm run validate:registry-determinism` wires the script.
`.github/workflows/ci.yml`: new `registry-determinism` job gated by `changes.outputs.code` (matches `.aiox-core/`, `scripts/`, `bin/`, `packages/`, `src/`, `tools/`). Runtime: ~5 min via existing ubuntu-latest runner. Blocks merge on drift.
Tested locally (both directions)
The remediation hint output ends with copy-paste-ready commands to fix locally:
```
node .aiox-core/development/scripts/populate-entity-registry.js
git add .aiox-core/data/entity-registry.yaml
git commit -m "chore(ids): regen entity-registry"
```
What this gate does NOT catch
Test plan
Issue
Closes #758
🤖 Generated with Claude Code
Summary by CodeRabbit